Improve handling of expr->field errors #124200

scrabsha · 2024-04-20T15:03:46Z

The current message for "-> used for field access" is the following:

error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `->`
 --> src/main.rs:2:6
  |
2 |     a->b;
  |      ^^ expected one of 8 possible tokens

(playground link)

This PR tries to address this by adding a dedicated error message and recovery. The proposed error message is:

error: `->` used for field access or method call
 --> ./tiny_test.rs:2:6
  |
2 |     a->b;
  |      ^^ help: try using `.` instead
  |
  = help: the `.` operator will dereference the value if needed

(feel free to bikeshed it as much as necessary)

rustbot · 2024-04-20T15:03:53Z

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

compiler/rustc_parse/src/parser/expr.rs

compiler-errors · 2024-04-20T15:25:07Z

compiler/rustc_parse/src/parser/expr.rs

+                // We make sure the `expr` and `->` are adjacent because we don't want to trigger the
+                // recovery for function signature.


Why would this happen?

Removing this makes issue-118530-ice.rs fail. I tried hard to understand this test. My understanding is that line 5 is a function signature, but I'd love to be corrected.

While avoiding this failure by checking the spans is not ideal, this indeed works and is consistent with how people space their -> in function prototypes and don't space their -> when accessing fields.

i feel pretty strongly that we shouldn't be using token connectedness to do this recovery -- if that test ICEs, it would be useful to understand why that happens and fix the recovery rather than just paper it over.

Also, i feel like removing the spaces around the -> in that test example should cause it to still ICE?

if so, this PR still introduces new bugs, and those should be fixed.

compiler/rustc_parse/src/parser/expr.rs

compiler/rustc_parse/messages.ftl

compiler-errors · 2024-04-20T17:00:09Z

also pls squash this into one commit

compiler/rustc_parse/messages.ftl

scrabsha · 2024-04-20T17:35:09Z

also pls squash this into one commit

@compiler-errors ah, sorry. I thought making multiple was helpful for the review. This won't happen again.

compiler-errors · 2024-04-20T17:36:09Z

It's literally nbd!

fmease · 2024-04-20T18:47:20Z

If I'm not mistaken, this PR regresses the following artificial code snippet:

Wait, no it shouldn't 🤦

compiler/rustc_parse/src/parser/expr.rs

The current message for "`->` used for field access" is the following: ```rust error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `->` --> src/main.rs:2:6 | 2 | a->b; | ^^ expected one of 8 possible tokens ``` (playground link[1]) This PR tries to address this by adding a dedicated error message and recovery. The proposed error message is: ``` error: `->` used for field access or method call --> ./tiny_test.rs:2:6 | 2 | a->b; | ^^ help: try using `.` instead | = help: the `.` operator will dereference the value if needed ``` (feel free to bikeshed it as much as necessary) [1]: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7f8b6f4433aa7866124123575456f54e Signed-off-by: Sasha Pourcelot <[email protected]>

scrabsha · 2024-04-22T15:53:26Z

@rustbot label -S-waiting-on-author +S-waiting-on-review

compiler-errors · 2024-04-22T15:55:56Z

👍 thanks for dealing w/ the back and forth

@bors r=compiler-errors,fmease

bors · 2024-04-22T15:55:59Z

📌 Commit 98332c1 has been approved by compiler-errors,fmease

It is now in the queue for this repository.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#123680 (Deny gen keyword in `edition_2024_compat` lints) - rust-lang#124057 (Fix ICE when ADT tail has type error) - rust-lang#124168 (Use `DefiningOpaqueTypes::Yes` in rustdoc, where the `InferCtxt` is guaranteed to have no opaque types it can define) - rust-lang#124197 (Move duplicated code in functions in `tests/rustdoc-gui/notable-trait.goml`) - rust-lang#124200 (Improve handling of expr->field errors) - rust-lang#124220 (Miri: detect wrong vtables in wide pointers) - rust-lang#124266 (remove an unused type from the reentrant lock tests) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#124200 - scrabsha:sasha/->, r=compiler-errors,fmease Improve handling of expr->field errors The current message for "`->` used for field access" is the following: ```rust error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `->` --> src/main.rs:2:6 | 2 | a->b; | ^^ expected one of 8 possible tokens ``` ([playground link](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7f8b6f4433aa7866124123575456f54e)) This PR tries to address this by adding a dedicated error message and recovery. The proposed error message is: ``` error: `->` used for field access or method call --> ./tiny_test.rs:2:6 | 2 | a->b; | ^^ help: try using `.` instead | = help: the `.` operator will dereference the value if needed ``` (feel free to bikeshed it as much as necessary)

rustbot assigned wesleywiser Apr 20, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 20, 2024

fmease reviewed Apr 20, 2024

View reviewed changes

compiler/rustc_parse/src/parser/expr.rs Outdated Show resolved Hide resolved

compiler-errors reviewed Apr 20, 2024

View reviewed changes

compiler/rustc_parse/src/parser/expr.rs Outdated Show resolved Hide resolved

compiler/rustc_parse/src/parser/expr.rs Outdated Show resolved Hide resolved

compiler/rustc_parse/messages.ftl Outdated Show resolved Hide resolved

compiler-errors reviewed Apr 20, 2024

View reviewed changes

compiler/rustc_parse/messages.ftl Outdated Show resolved Hide resolved

scrabsha force-pushed the sasha/-> branch from 0b2f314 to 9fbc9a8 Compare April 20, 2024 17:33

This comment was marked as resolved.

Sign in to view

fmease reviewed Apr 20, 2024

View reviewed changes

compiler/rustc_parse/src/parser/expr.rs Outdated Show resolved Hide resolved

fmease assigned compiler-errors and fmease and unassigned wesleywiser Apr 20, 2024

compiler-errors removed their assignment Apr 21, 2024

fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 21, 2024

scrabsha force-pushed the sasha/-> branch from 9fbc9a8 to 98332c1 Compare April 22, 2024 15:50

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 22, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 22, 2024

matthiaskrgr mentioned this pull request Apr 23, 2024

Rollup of 7 pull requests #124277

Merged

bors merged commit 5800e2a into rust-lang:master Apr 23, 2024

rustbot added this to the 1.79.0 milestone Apr 23, 2024

scrabsha deleted the sasha/-> branch April 23, 2024 10:54

		// We make sure the `expr` and `->` are adjacent because we don't want to trigger the
		// recovery for function signature.

Improve handling of expr->field errors #124200

Improve handling of expr->field errors #124200

Uh oh!

Conversation

scrabsha commented Apr 20, 2024 • edited by fmease Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 20, 2024

Uh oh!

Uh oh!

compiler-errors Apr 20, 2024

Choose a reason for hiding this comment

Uh oh!

scrabsha Apr 20, 2024

Choose a reason for hiding this comment

Uh oh!

compiler-errors Apr 20, 2024

Choose a reason for hiding this comment

Uh oh!

compiler-errors Apr 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

compiler-errors commented Apr 20, 2024

Uh oh!

Uh oh!

This comment was marked as resolved.

scrabsha commented Apr 20, 2024

Uh oh!

compiler-errors commented Apr 20, 2024

Uh oh!

fmease commented Apr 20, 2024

Uh oh!

Uh oh!

scrabsha commented Apr 22, 2024

Uh oh!

compiler-errors commented Apr 22, 2024

Uh oh!

bors commented Apr 22, 2024

Uh oh!

Uh oh!

scrabsha commented Apr 20, 2024 •

edited by fmease

Loading

compiler-errors Apr 20, 2024 •

edited

Loading